CodeFixService simplifications #59582
Conversation
|
|
||
| private readonly Func<Workspace, ImmutableDictionary<LanguageKind, Lazy<ImmutableDictionary<DiagnosticId, ImmutableArray<CodeFixProvider>>>>> _getWorkspaceFixersMap; |
There was a problem hiding this comment.
remove unnecessary callbacks. These can just be normal methods in this type.
| @@ -39,25 +39,23 @@ internal partial class CodeFixService : ICodeFixService | |||
| new((d1, d2) => DiagnosticId.CompareOrdinal(d1.Id, d2.Id)); | |||
|
|
|||
| private readonly IDiagnosticAnalyzerService _diagnosticService; | |||
| private readonly ImmutableArray<Lazy<CodeFixProvider, CodeChangeProviderMetadata>> _fixers; | |||
There was a problem hiding this comment.
don't store as an IEnumerable.
|
|
||
| private ImmutableDictionary<object, FixAllProviderInfo?> _fixAllProviderMap; | ||
| private ImmutableDictionary<CodeFixProvider, ImmutableArray<DiagnosticId>> _fixerToFixableIdsMap = ImmutableDictionary<CodeFixProvider, ImmutableArray<DiagnosticId>>.Empty; | ||
| private ImmutableDictionary<object, FixAllProviderInfo?> _fixAllProviderMap = ImmutableDictionary<object, FixAllProviderInfo?>.Empty; |
There was a problem hiding this comment.
move mutable fields to bottom.
CyrusNajmabadi
force-pushed
the
codeFixWork
branch
from
a6f8838
to
25a439f
Compare
|
|
||
| // REVIEW: currently, fixer's priority is statically defined by the fixer itself. might considering making it more dynamic or configurable. | ||
| _getFixerPriorityMap = workspace => GetFixerPriorityPerLanguageMap(fixersPerLanguageMap, workspace); | ||
| _lazyFixerToMetadataMap = new(() => fixers.Where(service => service.IsValueCreated).ToImmutableDictionary(service => service.Value, service => service.Metadata)); |
There was a problem hiding this comment.
Out of curiosity, when this runs in practice are there any fixers already created or is this just initializing an empty map?
There was a problem hiding this comment.
(why this is stuffed in a Lazy of its own is terrifying...)
There was a problem hiding this comment.
@mavasani this is a bit scary. this lazy is exposed through:
private ImmutableDictionary<CodeFixProvider, CodeChangeProviderMetadata> FixerToMetadataMap => _lazyFixerToMetadataMap.Value;This means that we capture whatever the state of hte world is when this is computed. Meaning if a service hasn't been created, then it's never in this map. This seems fishy, esp in a multi-language world. How do we know that we won't capture a snapshot of things in a bad state?
There was a problem hiding this comment.
attempting to fix that here: #59586
ghost
added this to the 
I'm working in this area, and makign a bunch of a changes. This extracts out some general cleanup that comes with no behavior changes.